I'm sorry but although this is a working solution for small datasets, for big ones it could be very problematic. The fix should be to make sure the reference isn't different in the first place.

Why is this needed?

Hi Iddan,

First off, it's commonly advised against including objects directly in the dependency array of a useEffect. This is because objects are compared by reference, not by value. What this means is that even if the content of the object hasn't changed, if a new object is created on each render, the effect will still run on every render because the references are different.

So, although comparing object references directly might seem like a good idea, it's actually not recommended.

In the case of react-spreadsheet, the code below can lead to an infinite loop due to shallow comparisons made within the useEffect body and the dependency array.

<Spreadsheet
  data={data}
  onChange={(newData: any) => {
    console.log(newData);
    setData(newData);
  }}
/>

React.useEffect(() => {
  if (state.model.data !== prevDataRef.current) {
    // Call onChange only if the data changes internally
    if (state.model.data !== props.data) {
      onChange(state.model.data);
    }
  }
  prevDataRef.current = state.model.data;
}, [state.model.data, onChange, props.data]);

To illustrate this point further, you can try changing the second block to:

React.useEffect(() => {
  console.log("Use Effect called");
}, [state.model.data]);

You'll notice that even though the object might appear to be the same instance, its reference changes after every rendering. This causes the console log to be called every time there is a modification.

To summarize:

1. state.model.data changes its reference every time setCellData is called.
2. Since the reference changes, the useEffect body is executed.
3. The first if condition is verified as the reference is always different from the one previously stored.
4. The second if condition is verified because there is no guarantee props.data has a reference equal to the new state.model.data reference.
5. The onChange function calls the setData function within the SpreadSheet object in App.tsx.
6. The loop restarts without an end.

Solution

Adding a new lastUpdateDate property addresses this issue while avoiding resource-intensive deep nested comparisons.

With this addition, we track the last time setCellData is executed. If the data displayed in the UI has a different lastUpdateDate value, the useEffect is called, and the displayed information is updated accordingly.

React.useEffect(() => {

}, [JSON.stringify(state.model.data)]);

Why not use just a string compare? For the huge datasets, it might be too slow though, but there is other libraries that can handle stringifying of an object more performant. Comparing a string, instead of having to traverse a huge object-graph is way easier.

The lastUpdate date check can probably cause some bad sideeffects as well.

Thank you @EdoardoGruppi for the detailed writing. I am unaware that any official React resources do not recommend object reference comparison. The real problem is the array reference changes which I don't see why it should happen more than once per mutation to the data.

@EdoardoGruppi are you using the latest version of the code? The controlled storybook story works correctly and it uses the same code as you posted.

@thsorens in the same spirit of React, I try to avoid unsimple comparisons on data.

Hi @iddan,
thanks for your responses.

I confirm that I am using the latter version of the code. To be 100% sure, I re-did the test by cloning the repo from scratch, but nothing changed. You can find a sandbox at this link to check the results I am getting. To see the logs, you just need to open the console in the embedded CodeSandbox browser.

You're correct that React doesn't explicitly advise against using non-primitive values inside the dependency array. However, many developers recommend avoiding this practice because it can lead to mistakes and unintended re-renders.

I am confident that this is the exact problem with the react-spreadsheet library. In one of my initial commits, I used a function to deep compare objects, which solved the issue. The same error can be highlighted when adding a log inside the outer if statement. The comparison state.model.data !== prevDataRef.current returns true endlessly every time a cell changes its value.

React.useEffect(() => {
  if (state.model.data !== prevDataRef.current) {
      console.log('state.model.data is not equal to prevDataRef.current')
  }
  prevDataRef.current = state.model.data;
}, [state.model.data]);

It's worth mentioning that I later replaced that function to avoid slowdowns with a comparison made on a new lastUpdatedDate property of the state object. I think that using the date is a nice way to solve the issue since: it doesn't add much code or memory occupation, makes the comparison very fast, and provides additional information that may be useful for future features.

@iddan : I also found a different weird behavior in the storybook version. Add some random text in a cell. Put the focus on another cell. Then go back to the cell, type 1 (the number 1). The focus will automatically disappear, so it will not be possible to continue writing in the cell. It might be related to the grid running in storybook, since i cant reproduce it in the "demo" version. (Running chrome)